Skip to content

Security rate limiting #165

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
harsh-hak wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Security rate limiting #165

harsh-hak wants to merge 6 commits into from

Conversation

@harsh-hak
Copy link

@harsh-hak harsh-hak commented Dec 16, 2025

I have implemented the following security enhancements:

  1. Auth Rate Limiter: Added a strict authLimiter (5 requests per 15 minutes) specifically designed for login and registration endpoints to mitigate brute-force and credential stuffing attacks.
  2. Centralized Logging: Refactored the security logging into a helper function logRateLimitExceeded. This ensures that all rate limit violations (Uploads, Auth, etc.) are logged consistently with IP addresses and context for security auditing.
  3. Response Hardening: Standardized the error response messages to be informative without leaking sensitive internal server details.

elanlaw1206
elanlaw1206 reviewed Dec 17, 2025
View reviewed changes
Copy link
Contributor

@elanlaw1206 elanlaw1206 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Harsh,

Thanks for the nice work. The auth rate limiter and centralized logging are solid security improvements, and the server boot refactor (require.main === module) is a good step for testability

A couple of small cleanups to consider (non-blocking):

  • This PR includes some unrelated Vulnerability_Tool artifacts and docs. It would be great to split those into a separate PR to keep this one focused on runtime security hardening.
  • There are a few dependency version changes mixed in, if possible, keeping them minimal or documenting why they’re needed here would help future reviews.
  • Minor: accessTokenExpiry is now 10m but the comment still says 15 minutes.

Overall, the security direction here looks good. Thanks for pushing this forward.

Thanks!
King Hei

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@elanlaw1206 elanlaw1206 elanlaw1206 left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@harsh-hak @elanlaw1206